Mixed choice/non-choice encoding#154
Merged
MaxDesiatov merged 15 commits intoCoreOffice:masterfrom Nov 27, 2019
Merged
Conversation
MaxDesiatov
requested changes
Nov 26, 2019
Comment on lines
118
to
119
| // We haven't yet pushed a container at this level; do so here. | ||
| topContainer = storage.pushChoiceContainer() | ||
| let container = XMLChoiceEncodingContainer<Key>( |
Comment on lines
77
to
78
| if canEncodeNewValue { | ||
| // We haven't yet pushed a container at this level; do so here. | ||
| topContainer = storage.pushKeyedContainer() | ||
| let container = XMLKeyedEncodingContainer<Key>( |
Collaborator
There was a problem hiding this comment.
the comment above seems to be outdated now
| Attempt to push new keyed encoding container when already previously encoded \ | ||
| at this path. | ||
| """ | ||
| ) |
Collaborator
There was a problem hiding this comment.
Overall this switch seems to be a duplicate of switch in public func keyedContainer modified above, could it be generalized somehow?
Collaborator
Author
There was a problem hiding this comment.
Ah, good point!
houndci-bot
reviewed
Nov 26, 2019
MaxDesiatov
approved these changes
Nov 27, 2019
Collaborator
MaxDesiatov
left a comment
There was a problem hiding this comment.
Amazing, many thanks!
MaxDesiatov
pushed a commit
that referenced
this pull request
Dec 1, 2019
Mirrors #154 Fixes bugs 1) Decoding multiple choice elements in the same Keyed Container. 2) Decoding choice elements after decoding regular keyed elements in the same container. Case 1 refers to decoding implementations of the form: ```swift init(from decoder: Decoder) throws { let container = try decoder.container(keyedBy: CodingKeys.self) otherValue = try container.decode(String.self, forKey: .otherValue) intOrString = try IntOrString(from: decoder) } ``` where `IntOrString` is a choice coding element `IntOrString` defined as follows: ```swift enum IntOrString { case int(Int) case string(String) } extension IntOrString: Decodable { enum CodingKeys: String, CodingKey { case int case string } init(from decoder: Decoder) throws { let container = try decoder.container(keyedBy: CodingKeys.self) if container.contains(.int) { self = .int(try container.decode(Int.self, forKey: .int)) } else { self = .string(try container.decode(String.self, forKey: .string)) } } } extension IntOrString.CodingKeys: XMLChoiceCodingKey {} // signifies that `IntOrString` is a choice element ``` Case 2 refers to decoding implementations of the following form: ```swift init(from decoder: Decoder) throws { self.first = try IntOrString(from: decoder) self.second = try AlternateIntOrString(from: decoder) } ``` Where both `first: IntOrString` and `second: AlternateIntOrString` are choice elements. `AlternateIntOrString` defined as follows: ```swift private enum AlternateIntOrString { case alternateInt(Int) case alternateString(String) } extension AlternateIntOrString: Decodable { enum CodingKeys: String, CodingKey { case alternateInt = "alternate-int" case alternateString = "alternate-string" } init(from decoder: Decoder) throws { let container = try decoder.container(keyedBy: CodingKeys.self) if container.contains(.alternateInt) { self = .alternateInt(try container.decode(Int.self, forKey: .alternateInt)) } else { self = .alternateString(try container.decode(String.self, forKey: .alternateString)) } } } extension AlternateIntOrString.CodingKeys: XMLChoiceCodingKey {} // signifies that `AlternateIntOrString` is a choice element ``` * Add tests * Fix failing tests
arjungupta0107
pushed a commit
to salido/XMLCoder
that referenced
this pull request
Jun 26, 2020
## Overview
Fixes bug encountered when encoding structs that hold a mixture of choice-element and non-choice-element (or multiple choice-element) properties.
## Example
Given a structure that stores both a choice and non-choice property,
```swift
private struct IntOrStringAndDouble: Equatable {
let intOrString: IntOrString
let decimal: Double
}
```
the natural encoding approach (now available) is
```swift
extension IntOrStringAndDouble: Encodable {
enum CodingKeys: String, CodingKey {
case decimal
}
func encode(to encoder: Encoder) {
try intOrString.encode(to: encoder)
var container = encoder.container(keyedBy: CodingKeys.self)
try container.encode(decimal, forKey: .decimal)
}
}
```
The following `encode` implementation also works:
```swift
extension IntOrStringAndDouble: Encodable {
enum CodingKeys: String, CodingKey {
case decimal
}
func encode(to encoder: Encoder) {
var singleValueContainer = encoder.singleValueContainer()
try singleValueContainer.encode(intOrString)
var container = encoder.container(keyedBy: CodingKeys.self)
try container.encode(decimal, forKey: .decimal)
}
}
```
`IntOrString` as defined in CoreOffice#119:
```swift
enum IntOrString: Equatable {
case int(Int)
case string(String)
}
extension IntOrString: Encodable {
enum CodingKeys: String, CodingKey {
case int
case string
}
func encode(to encoder: Encoder) throws {
var container = encoder.container(keyedBy: CodingKeys.self)
switch self {
case let .int(value):
try container.encode(value, forKey: .int)
case let .string(value):
try container.encode(value, forKey: .string)
}
}
}
extension IntOrString.CodingKeys: XMLChoiceCodingKey {} // signifies that `IntOrString` is a choice element
```
## Implementation Details
In cases where choice and non-choice elements (or multiple choice elements) co-exist in a keyed container, we merge them into a single `XMLKeyedEncodingContainer` (wrapping a `SharedBox<KeyedBox>`). Arrays of choice elements (using `XMLUnkeyedEncodingContainer` under the hood) are encoded the same way as before, as we do not hit the merging cases. For the array case, we still need the `XMLChoiceEncodingContainer` structure.
## Source Compatibility
This is an additive change.
* Add breaking case
* Add choice and keyed merging encode functionality
* Refactor
* Fix commented code
* Fix misnamed file
* Fix xcode project
* Fix precondition catch
* Use switch syntax
* Add multiple choice element case
* Add explicit types in KeyedBox initialization
* Add explicitly empty parameter to KeyedBox initializer
* Use more concise type inference
* Unify switch syntax
* Cut down code duplication
* Fix formatting
arjungupta0107
pushed a commit
to salido/XMLCoder
that referenced
this pull request
Jun 26, 2020
Mirrors CoreOffice#154 Fixes bugs 1) Decoding multiple choice elements in the same Keyed Container. 2) Decoding choice elements after decoding regular keyed elements in the same container. Case 1 refers to decoding implementations of the form: ```swift init(from decoder: Decoder) throws { let container = try decoder.container(keyedBy: CodingKeys.self) otherValue = try container.decode(String.self, forKey: .otherValue) intOrString = try IntOrString(from: decoder) } ``` where `IntOrString` is a choice coding element `IntOrString` defined as follows: ```swift enum IntOrString { case int(Int) case string(String) } extension IntOrString: Decodable { enum CodingKeys: String, CodingKey { case int case string } init(from decoder: Decoder) throws { let container = try decoder.container(keyedBy: CodingKeys.self) if container.contains(.int) { self = .int(try container.decode(Int.self, forKey: .int)) } else { self = .string(try container.decode(String.self, forKey: .string)) } } } extension IntOrString.CodingKeys: XMLChoiceCodingKey {} // signifies that `IntOrString` is a choice element ``` Case 2 refers to decoding implementations of the following form: ```swift init(from decoder: Decoder) throws { self.first = try IntOrString(from: decoder) self.second = try AlternateIntOrString(from: decoder) } ``` Where both `first: IntOrString` and `second: AlternateIntOrString` are choice elements. `AlternateIntOrString` defined as follows: ```swift private enum AlternateIntOrString { case alternateInt(Int) case alternateString(String) } extension AlternateIntOrString: Decodable { enum CodingKeys: String, CodingKey { case alternateInt = "alternate-int" case alternateString = "alternate-string" } init(from decoder: Decoder) throws { let container = try decoder.container(keyedBy: CodingKeys.self) if container.contains(.alternateInt) { self = .alternateInt(try container.decode(Int.self, forKey: .alternateInt)) } else { self = .alternateString(try container.decode(String.self, forKey: .alternateString)) } } } extension AlternateIntOrString.CodingKeys: XMLChoiceCodingKey {} // signifies that `AlternateIntOrString` is a choice element ``` * Add tests * Fix failing tests
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Overview
Fixes bug encountered when encoding structs that hold a mixture of choice-element and non-choice-element (or multiple choice-element) properties.
Example
Given a structure that stores both a choice and non-choice property,
the natural encoding approach (now available) is
The following
encodeimplementation also worksIntOrStringas defined in #119Implementation Details
In cases where choice and non-choice elements (or multiple choice elements) co-exist in a keyed container, we merge them into a single
XMLKeyedEncodingContainer(wrapping aSharedBox<KeyedBox>). Arrays of choice elements (usingXMLUnkeyedEncodingContainerunder the hood) are encoded the same way as before, as we do not hit the merging cases. For the array case, we still need theXMLChoiceEncodingContainerstructure.Source Compatibility
This is an additive change.